-
Notifications
You must be signed in to change notification settings - Fork 899
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use ActiveSupport Concern #289
Conversation
Can you give an example where you want to use this? |
# with concerns - able to inherit custom Base class and share connections
module Foo
class Base < ActiveRecord::Base
end
class Version < Base
include PaperTrail::VersionConcern
end
class Document < Base
has_paper_trail class_name: 'Foo::Version'
end
end
module Bar
class Base < ActiveRecord::Base
end
class Version < Base
include PaperTrail::VersionConcern
end
class Document < Base
has_paper_trail class_name: 'Bar::Version'
end
end
Foo::Base.establish_connection({})
Bar::Base.establish_connection({}) Without concerns, version classes need to have it's own connection because PaperTrail::Version will use ActiveRecord::Base's connection. In addition it's difficult to add common behaviors or override ActiveRecord::Base method on Foo::Version using Foo::Base, because it inherits PaperTrail::Version (and ActiveRecord::Base), not Foo:Base. # without concerns - need to inherit PaperTrail::Version
module Foo
class Version < PaperTrail::Version
end
end
module Bar
class Version < PaperTrail::Version
end
end
[Foo::Base, Foo::Version].each { |klass| klass.establish_connection({database: 'foo'}) }
[Bar::Base, Bar::Version].each { |klass| klass.establish_connection({database: 'bar'}) } |
So essentially this just makes it easy to use the gem with an application that makes multiple connections to databases. I think this makes sense then since it leaves the original functionality in tact but adds some flexibility. Any chance you could add a test or spec demonstrating the functionality if it's not too much trouble? If nothing else, it makes the use case easy to see for others going forward. |
FYI it's idiomatic usage of the AS::Concern module to define your class-level methods in a module called module VersionConcern
included do
# macros in here
end
module ClassMethods
def some_class_level_method
end
end
def some_instance_level_method
end
end |
@jherdman I'll make that change - thanks! |
It makes easy to use version class with different AR connection
@chulkilee - Probably just something similar to your example above, where a custom version class includes the Also, it looks like the portion with the class methods needs to be modified a bit. The |
including missed class methods
@fullbridge-batkins fixed and added tests |
Yes I saw that, thank you, looks good. Was busy last week but going to merge this soon. |
Thanks a lot for this pull, very cool feature. I'm wondering if there is some way to document the usage of this on the |
thanks! |
It makes easy to use version class with different AR connection